-
Notifications
You must be signed in to change notification settings - Fork 421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add GPU sync tests #2194
add GPU sync tests #2194
Conversation
This pull request was exported from Phabricator. Differential Revision: D59173140 |
This pull request was exported from Phabricator. Differential Revision: D59173140 |
ef38e07
to
7ecf930
Compare
Summary: Pull Request resolved: pytorch#2194 Added GPU sync tests to simulate gathering metric states on to rank 0 and computing. Tests don't cover this case before, which has resulted in SEVs in the past as users aren't aware of how RecMetrics collects and computes metrics. Added numerical value tests, most metrics are do not have this which can result in issues down the line if metrics need to be changed/accomodate future changes. Also we've found inconsistencies sometimes from other methods, so always good to check here. We compare each metric to a reference implementation from literature to ensure the values are as expected. Reviewed By: henrylhtsang Differential Revision: D59173140
Summary: Added GPU sync tests to simulate gathering metric states on to rank 0 and computing. Tests don't cover this case before, which has resulted in SEVs in the past as users aren't aware of how RecMetrics collects and computes metrics. Added numerical value tests, most metrics are do not have this which can result in issues down the line if metrics need to be changed/accomodate future changes. Also we've found inconsistencies sometimes from other methods, so always good to check here. We compare each metric to a reference implementation from literature to ensure the values are as expected. Reviewed By: henrylhtsang Differential Revision: D59173140
7ecf930
to
b15a18a
Compare
This pull request was exported from Phabricator. Differential Revision: D59173140 |
Summary: Added GPU sync tests to simulate gathering metric states on to rank 0 and computing. Tests don't cover this case before, which has resulted in SEVs in the past as users aren't aware of how RecMetrics collects and computes metrics. Added numerical value tests, most metrics are do not have this which can result in issues down the line if metrics need to be changed/accommodate future changes. Also we've found inconsistencies sometimes from other methods, so always good to check here. We compare each metric to a reference implementation from literature to ensure the values are as expected. Fixed and cleaned up some tests too. Reviewed By: henrylhtsang Differential Revision: D59173140
b15a18a
to
b482bb5
Compare
This pull request was exported from Phabricator. Differential Revision: D59173140 |
b482bb5
to
b1707f5
Compare
Summary: Added GPU sync tests to simulate gathering metric states on to rank 0 and computing. Tests don't cover this case before, which has resulted in SEVs in the past as users aren't aware of how RecMetrics collects and computes metrics. Added numerical value tests, most metrics are do not have this which can result in issues down the line if metrics need to be changed/accommodate future changes. Also we've found inconsistencies sometimes from other methods, so always good to check here. We compare each metric to a reference implementation from literature to ensure the values are as expected. Fixed and cleaned up some tests too. Reviewed By: henrylhtsang Differential Revision: D59173140
This pull request was exported from Phabricator. Differential Revision: D59173140 |
b1707f5
to
1356d1d
Compare
Summary: Added GPU sync tests to simulate gathering metric states on to rank 0 and computing. Tests don't cover this case before, which has resulted in SEVs in the past as users aren't aware of how RecMetrics collects and computes metrics. Added numerical value tests, most metrics are do not have this which can result in issues down the line if metrics need to be changed/accommodate future changes. Also we've found inconsistencies sometimes from other methods, so always good to check here. We compare each metric to a reference implementation from literature to ensure the values are as expected. Fixed and cleaned up some tests too. Reviewed By: henrylhtsang Differential Revision: D59173140
This pull request was exported from Phabricator. Differential Revision: D59173140 |
1356d1d
to
65f08fe
Compare
Summary: **TLDR: Significant clean up of recmetrics tests and fixing long standing testing issues with particular metrics. Because RecMetrics is community sourced, the quality of metric and test implementations can have high variance which leads to SEVs later on due to the high surface area RecMetrics operates on.** Added GPU sync tests to simulate gathering metric states on to rank 0 and computing. Tests don't cover this case before, which has resulted in SEVs in the past as users aren't aware of how RecMetrics collects and computes metrics. Added numerical value tests, most metrics are do not have this which can result in issues down the line if metrics need to be changed/accommodate future changes. Also we've found inconsistencies sometimes from other methods, so always good to check here. We compare each metric to a reference implementation from literature to ensure the values are as expected. Fixed and cleaned up some tests, particularly enforcing a standard of quality that can be referenced for future metric implementations and tests. Reviewed By: henrylhtsang Differential Revision: D59173140
This pull request was exported from Phabricator. Differential Revision: D59173140 |
Summary: **TLDR: Significant clean up of recmetrics tests and fixing long standing testing issues with particular metrics. Because RecMetrics is community sourced, the quality of metric and test implementations can have high variance which leads to SEVs later on due to the high surface area RecMetrics operates on.** Added GPU sync tests to simulate gathering metric states on to rank 0 and computing. Tests don't cover this case before, which has resulted in SEVs in the past as users aren't aware of how RecMetrics collects and computes metrics. Added numerical value tests, most metrics are do not have this which can result in issues down the line if metrics need to be changed/accommodate future changes. Also we've found inconsistencies sometimes from other methods, so always good to check here. We compare each metric to a reference implementation from literature to ensure the values are as expected. Fixed and cleaned up some tests, particularly enforcing a standard of quality that can be referenced for future metric implementations and tests. Reviewed By: henrylhtsang Differential Revision: D59173140
65f08fe
to
6c353a8
Compare
This pull request was exported from Phabricator. Differential Revision: D59173140 |
Summary:
TLDR: Significant clean up of recmetrics tests and fixing long standing testing issues with particular metrics. Because RecMetrics is community sourced, the quality of metric and test implementations can have high variance which leads to SEVs later on due to the high surface area RecMetrics operates on.
Added GPU sync tests to simulate gathering metric states on to rank 0 and computing. Tests don't cover this case before, which has resulted in SEVs in the past as users aren't aware of how RecMetrics collects and computes metrics.
Added numerical value tests, most metrics are do not have this which can result in issues down the line if metrics need to be changed/accommodate future changes. Also we've found inconsistencies sometimes from other methods, so always good to check here. We compare each metric to a reference implementation from literature to ensure the values are as expected.
Fixed and cleaned up some tests, particularly enforcing a standard of quality that can be referenced for future metric implementations and tests.
Reviewed By: henrylhtsang
Differential Revision: D59173140